perf: Migrate to Flashlist in MoneyRequestReportTransactionList#91422
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
TMisiukiewicz
changed the title
|
@aimane-chnaif Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b5c9bef51
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
5c17a83 to
a71286a
Compare
|
@aimane-chnaif addressed all valid comments 🙏 |
…rt-list # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx
|
@aimane-chnaif resolved conflicts, ready for final review |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
570b3af to
8807db4
Compare
aimane-chnaif
left a comment
aimane-chnaif
left a comment
There was a problem hiding this comment.
I did regression tests as much as I could.
We'll see what bugs QA team will find in staging.
|
I think this was mentioned earlier but empty view briefly shows when fast scroll. Let's not block on this. Screen.Recording.2026-07-22.at.4.17.32.pm.mov
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8807db4866
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| case 'transactions-footer': | ||
| return controller.afterListContent; | ||
| case 'report-action': | ||
| return renderReportAction(item.action, index - reportActionIndexOffset); |
There was a problem hiding this comment.
When transactions are inlined ahead of chat actions, this subtracts reportActionIndexOffset before the action reaches ReportActionIndexContext. That context is used by ReportActionItemMessageEdit to call reportScrollManager.scrollToIndex(index) on mobile Chrome when the edit composer focuses, so in a money-request report with inline transactions the scroll target becomes a transaction row (or another earlier item) instead of the edited comment. Keep a separate local action index for grouping, but provide the raw FlashList index to context consumers that scroll the unified list.
Useful? React with 👍 / 👎.
|
🤖 Regression-focused review of the FlashList migration. This is a large, high-leverage change (unified virtualized list + two patches to the shared 🔴 Highest risk — the
|
|
|
||
| - Reason: Adds an **`overrideWindowSize`** prop that lets a list declare its visible window (`{width, height}`) instead of deriving it from `measureParentSize(internalViewRef)`. Needed for an *externally-driven* list — one whose `renderScrollComponent` is a non-scrolling `View` that grows to the full content height and receives synthetic scroll events from a parent scroller. Without this, FlashList measures the outer container (as tall as all content) as its viewport and renders every row, defeating virtualization. The change is minimal: `measureParentSize` is still assigned to `outerViewSize` and used for `containerViewSizeRef` (layout-change detection) and the 0×0 hidden-guard from patch 002; only the `windowSize` fed to `updateLayoutParams` is `overrideWindowSize ?? outerViewSize`. Fully backward compatible — when the prop is unset, `windowSize === outerViewSize` and behavior is byte-identical. Used by `MoneyRequestReportView`'s horizontally-scrollable transaction table (`ExternalScrollFlashListTable`), which windows its rows against the unified list's vertical scroll. | ||
| - Files changed: `src/FlashListProps.ts`, `src/recyclerview/RecyclerView.tsx`, `dist/FlashListProps.d.ts`, `dist/recyclerview/RecyclerView.js`. | ||
| - Upstream PR/issue: TBD |
There was a problem hiding this comment.
If we really need this patch, lets create upstream issue as well. For both
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Explanation of Change
Problem
On heavy money-request reports the RHP list was effectively non-virtualised. The inner transactions section was rendered via plain
.map()inside the parent FlatList'sListHeaderComponent, which renders eagerly. Every transaction row mounted synchronously when the report opened, pushing report open time to ~20–30s for a report with 1600 expenses and producing a noticeable jank window on every navigation.Solution
Unified list — merged the inner transactions section and the outer report-actions section into a single virtualised list. A discriminated-union
UnifiedListItem(section-header|transaction|transactions-footer|report-action) flows through onerenderItemdispatcher, so transactions and chat messages now share the same recycler window instead of one being eagerly rendered inside the other's header.FlashList — migrated the list from
react-native'sFlatList(FlatListWithScrollKey).initialScrollIndexreplaces the FlatList-specificuseFlatListScrollKeywrapperController + sub-component —
MoneyRequestReportTransactionListcontinues to own the transaction-domain state (sort, group-by, selection, columns, totals, etc.) and exposes it to the parent via a render-prop controller. The parent renders a smallMoneyRequestReportUnifiedListsub-component that assembles the FlashList from controller data plus the report-actions listPerformance numbers (web):
1.6k transactions: 15s -> 660ms
32 transactions: 460ms -> 400ms
128 transactions: 1.6s -> 431ms
192 transactions: 2.3s -> 481ms
Fixed Issues
$ #91425
PROPOSAL:
Tests
Before testing:
npm run i-standaloneas this PR contains patch to the FlashListOffline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
ios.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
web.mov